-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bugfix: Make sure vacuum works on relative paths #664
Conversation
.map(|rel_path| self.storage.join_path(&self.table_uri, rel_path)) | ||
.collect::<Vec<_>>(); | ||
match self.storage.delete_objs(paths).await { | ||
match self.storage.delete_objs(&files_to_delete).await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this was working in object stores, but in local fs files_to_delete
was already absolute. Local file system used Path::join()
, which had the fortunate behavior of just using the second path if it was already absolute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... maybe its not :) - will have a closer look once i get to #647.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
.map(|rel_path| self.storage.join_path(&self.table_uri, rel_path)) | ||
.collect::<Vec<_>>(); | ||
match self.storage.delete_objs(paths).await { | ||
match self.storage.delete_objs(&files_to_delete).await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... maybe its not :) - will have a closer look once i get to #647.
* chore: create failing test showing vacuum doesn't work on relative paths * fix: don't needlessly join paths
Description
It seems that when we pass a relative path to
DeltaTable()
, vacuum doesn't actually delete files. I've added tests in Python which is more convenient now that we have a writer.Related Issue(s)
Documentation